Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A flag that checks if set-camera-host sexp should use the model's eyepoint for the camera position when no submodel is specified #6540

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LuytenKy
Copy link

No description provided.

…point for the camera position when no submodel is specificed
Copy link
Contributor

@Goober5000 Goober5000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you open a pull request to address an existing issue, you should link to that issue in the description. So, edit the description to contain the text #6509. That will automatically create a link.

Also, several items in the code:

@@ -502,6 +503,12 @@ void parse_mod_table(const char *filename)
}
}

if (optional_string("$Use model eyepoint for set-camera-facing:")) {
stuff_boolean(&Use_model_eyepoint_for_set_camera_facing);
if (Use_model_eyepoint_for_set_camera_facing) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an opening brace at the end of this line without a closing brace. An if that contains a single line doesn't need braces, but if you add them, there needs to be a pair.

@@ -1667,6 +1675,7 @@ void mod_table_set_version_flags()
Shockwaves_damage_all_obj_types_once = true;
Framerate_independent_turning = true; // this is already true, but let's re-emphasize it
Use_host_orientation_for_set_camera_facing = true; // this is essentially a bugfix
Use_model_eyepoint_for_set_camera_facing = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good to add this flag as a bugfix, as you did here, but it's in the wrong section. Any new flags added should be associated with the upcoming stable version, so this should be added in the 25.0.0 section. (Currently, that section doesn't exist, so you'll need to create it.)

@@ -42,6 +42,7 @@ extern bool Cutscene_camera_displays_hud;
extern bool Alternate_chaining_behavior;
extern bool Fixed_chaining_to_repeat;
extern bool Use_host_orientation_for_set_camera_facing;
extern bool Use_model_eyepoint_for_set_camera_facing;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the issue you are addressing is to use the model eyepoint for set-camera-host, both the variable and the text parsed in game_settings.tbl should refer to set-camera-host, not set-camera-facing.

@@ -314,6 +314,24 @@ void camera::set_rotation_facing(vec3d *in_target, float in_rotation_time, float
matrix orient_buf;
matrix *orient = nullptr;

if (Use_model_eyepoint_for_set_camera_facing) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the wrong place for the change in behavior. Since the flag is supposed to modify the functionality of set-camera-host, take a look at that sexp implementation and the functions called by it.

(If you still need help after that hint, I can give more specific information.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants